refactor: use some colors in CLI output#962
Conversation
dosubot
Bot
added
the
size:XL
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR updates tinyauth CLI commands to print styled YAML-oriented output, adds shared CLI rendering helpers, changes YAML tags on exported config structs to ChangesCLI command output and error handling refactor
Config struct YAML tag updates
JetBrains ignore rule
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
cmd/tinyauth/tinyauth.go (3)
166-169: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFatal errors are written to stdout, not stderr.
fatalfusesfmt.Printf, sending error output to stdout. Conventionally, fatal/error messages should go to stderr so they aren't mixed with normal output when stdout is redirected/piped.🛠️ Suggested fix
func fatalf(err error, msg string) { - fmt.Printf("%s: %v\n", msg, err) + fmt.Fprintf(os.Stderr, "%s: %v\n", msg, err) os.Exit(1) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tinyauth/tinyauth.go` around lines 166 - 169, fatalf is printing fatal error messages to stdout via fmt.Printf, which should be changed to stderr. Update fatalf in tinyauth.go to write the error message using stderr output instead of stdout, keeping the same message format and os.Exit(1) behavior so normal program output is not mixed with error output.
133-135: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent no-op on "is not runnable".
When a command is not runnable (e.g. a group command invoked without a subcommand), the program silently returns with no output at all. Consider printing help or a short hint so the user isn't left staring at a blank prompt.
💡 Suggested fix
if strings.Contains(err.Error(), "is not runnable") { + _ = cmdTinyauth.PrintHelp(os.Stdout) return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tinyauth/tinyauth.go` around lines 133 - 135, The error handling in tinyauth.go currently returns silently when err.Error() contains "is not runnable", which leaves users with no feedback. Update the command execution path around the existing err check to emit a short hint or the command help instead of returning immediately, using the same command object or help-related logic in the surrounding handler so group commands without subcommands still produce output.
60-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated
AddCommand+ error-check boilerplate.The
err := ...; if err != nil { fatalf(...) }pattern is repeated 10 times. A small helper would reduce duplication and the risk of a copy-paste mistake (e.g. wrong error message for the wrong command).♻️ Suggested helper
func mustAddCommand(parent, child *cli.Command, failMsg string) { if err := parent.AddCommand(child); err != nil { fatalf(err, failMsg) } }Then each call site becomes e.g.
mustAddCommand(cmdTinyauth, helpCmd, "Failed to add help command").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tinyauth/tinyauth.go` around lines 60 - 124, The command registration in tinyauth setup repeats the same AddCommand error-handling pattern many times. Introduce a small helper around AddCommand, using the existing cmdTinyauth/cmdUser/cmdTotp/cmdOidc registration flow, so each command is added through one shared function that calls fatalf on error with the provided message. This will remove the duplicated err checks and reduce the chance of copy-paste mistakes in the command setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/tinyauth/config.go`:
- Line 15: The help text for the config dump command is stale: the Description
in config.go still says JSON even though the command now emits YAML via
yaml.Marshal and the user-facing message in the config command says YAML. Update
the command metadata in the config command setup so the Description matches the
actual output format, and keep it consistent with the config dumping logic in
the config command implementation.
- Around line 29-41: The YAML line coloring logic in the config rendering path
incorrectly splits every line on the first colon, which breaks scalar list items
like "- value" entries that contain colons in their content. Update the
rendering logic around the loop in config.go to skip colon-splitting for list
items and only treat actual mapping lines as key/value pairs, and apply the same
fix to the duplicated implementation in create_oidc_client.go. If possible,
extract the shared formatting into a helper such as renderYAMLToBuf so both call
sites use the same corrected behavior.
In `@cmd/tinyauth/create_user.go`:
- Around line 89-98: The credential formatting in create_user.go is escaping `$`
twice and in the wrong places, so the Docker-specific output and the non-Docker
output diverge incorrectly. In the create_user flow, keep the raw `user` value
(built from tCfg.Username and passwdStr) for the YAML/example section, and apply
the `strings.ReplaceAll(..., "$", "$$")` transformation only once to the env
var/CLI value that becomes `escapedUser`. Make sure the `tCfg.Docker` branch
affects only the docker-compose-safe string and does not alter the raw hash used
elsewhere.
In `@cmd/tinyauth/tinyauth.go`:
- Around line 171-178: The output order in renderToBuf is non-deterministic
because it ranges over a map, so update renderToBuf to accept an ordered slice
of key/value pairs instead of kv map[string]string and iterate in that provided
order. Then adjust the callers in create_user.go and create_oidc_client.go to
build and pass an ordered list so the rendered CLI output is stable across runs.
---
Nitpick comments:
In `@cmd/tinyauth/tinyauth.go`:
- Around line 166-169: fatalf is printing fatal error messages to stdout via
fmt.Printf, which should be changed to stderr. Update fatalf in tinyauth.go to
write the error message using stderr output instead of stdout, keeping the same
message format and os.Exit(1) behavior so normal program output is not mixed
with error output.
- Around line 133-135: The error handling in tinyauth.go currently returns
silently when err.Error() contains "is not runnable", which leaves users with no
feedback. Update the command execution path around the existing err check to
emit a short hint or the command help instead of returning immediately, using
the same command object or help-related logic in the surrounding handler so
group commands without subcommands still produce output.
- Around line 60-124: The command registration in tinyauth setup repeats the
same AddCommand error-handling pattern many times. Introduce a small helper
around AddCommand, using the existing cmdTinyauth/cmdUser/cmdTotp/cmdOidc
registration flow, so each command is added through one shared function that
calls fatalf on error with the provided message. This will remove the duplicated
err checks and reduce the chance of copy-paste mistakes in the command setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f2c4b94-9b3b-4ec6-a0c6-4c868b5232ef
📒 Files selected for processing (8)
cmd/tinyauth/config.gocmd/tinyauth/create_oidc_client.gocmd/tinyauth/create_user.gocmd/tinyauth/generate_totp.gocmd/tinyauth/tinyauth.gocmd/tinyauth/verify_user.gocmd/tinyauth/version.gointernal/model/config.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/tinyauth/create_oidc_client.go (1)
71-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse dot-separated OIDC flag paths
flag.Decodemaps nested config with dot notation, so these examples should be--oidc.clients.<name>.<field>instead of the current hyphenated names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tinyauth/create_oidc_client.go` around lines 71 - 86, The CLI flag examples in create_oidc_client.go use hyphenated names, but they should match flag.Decode’s nested dot notation. Update the renderToBuf entries in the CLI flags section to use the OIDC flag path format with dots, referencing the existing lclientName-based keys and the related clientId, clientSecret, and utils.Capitalize values so the generated examples align with the actual config structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@cmd/tinyauth/create_oidc_client.go`:
- Around line 71-86: The CLI flag examples in create_oidc_client.go use
hyphenated names, but they should match flag.Decode’s nested dot notation.
Update the renderToBuf entries in the CLI flags section to use the OIDC flag
path format with dots, referencing the existing lclientName-based keys and the
related clientId, clientSecret, and utils.Capitalize values so the generated
examples align with the actual config structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 70cf0825-f130-41a9-8e75-ad31ca36067f
📒 Files selected for processing (5)
cmd/tinyauth/config.gocmd/tinyauth/create_oidc_client.gocmd/tinyauth/create_user.gocmd/tinyauth/tinyauth.gointernal/model/config.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/tinyauth/tinyauth.go
- internal/model/config.go
2 participants
Summary by CodeRabbit
helpsubcommand and improvedversionoutput styling.config,create,generate,verify, and OIDC client generation to show YAML-focused, styled console output; OIDC client flags renamed to--oidc-clients-<name>-*.omitempty) in generated YAML (except a Tailscale-related field).